Skip to content

Custom function and script responses #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Oct 13, 2025
Merged

Custom function and script responses #216

merged 15 commits into from
Oct 13, 2025

Conversation

adam-fowler
Copy link
Collaborator

@adam-fowler adam-fowler commented Sep 12, 2025 •
edited
Loading

Code generator will attempt to choose the correct return value for a Valkey command, but it can only do simple types like String, Int, Array of RESP Tokens. If we want to provide more complex responses we have to write some custom code. To do this

  • disable the return value generation for that command in the code generator
  • override the Response typealias in the ValkeyCommand type
  • provide a method to initialise this Response from a RESPToken.

Also in this PR

  • moved integration tests, testing commands into separate file
  • Added command test helper testCommandEncodesDecodes and use in script and server commands, will extend to others in separate PR

@adam-fowler
Custom function and script responses
9e05b31 
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
@adam-fowler adam-fowler marked this pull request as draft September 12, 2025 16:28
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 12, 2025 •
edited
Loading

✅ Pull request no significant performance differences ✅

Summary

New baseline 'pull_request' is WITHIN the 'main' baseline thresholds.

Full Benchmark Comparison

Comparing results between 'main' and 'pull_request'

Host 'ea765db6c436' with 4 'x86_64' processors with 15 GB memory, running:
#18~24.04.1-Ubuntu SMP Sat Jun 28 04:46:03 UTC 2025

ValkeyBenchmarks

Client: GET benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 82 85 85 88 88 88 88 6
pull_request 81 82 83 86 90 90 90 6
Δ -1 -3 -2 -2 2 2 2 0
Improvement % 1 4 2 2 -2 -2 -2 0

Client: GET benchmark | parallel 20 | 20 concurrent connections metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 97 100 103 106 108 113 113 25
pull_request 96 99 103 106 108 119 119 24
Δ -1 -1 0 0 0 6 6 -1
Improvement % 1 1 0 0 0 -5 -5 -1

Connection: GET benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 4 4 4 4 4 4 4 8
pull_request 4 4 4 4 4 4 4 9
Δ 0 0 0 0 0 0 0 1
Improvement % 0 0 0 0 0 0 0 1

Connection: GET benchmark – NoOpTracer metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 7 8 8 10 10 10 10 8
pull_request 7 8 8 10 10 10 10 8
Δ 0 0 0 0 0 0 0 0
Improvement % 0 0 0 0 0 0 0 0

Connection: Pipeline array benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 33 33 33 34 34 34 34 6
pull_request 33 33 33 34 34 34 34 6
Δ 0 0 0 0 0 0 0 0
Improvement % 0 0 0 0 0 0 0 0

Connection: Pipeline benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 33 33 33 34 34 34 34 6
pull_request 33 33 33 34 34 34 34 6
Δ 0 0 0 0 0 0 0 0
Improvement % 0 0 0 0 0 0 0 0

HashSlot – {user}.whatever metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 17
pull_request 0 0 0 0 0 0 0 17
Δ 0 0 0 0 0 0 0 0
Improvement % 0 0 0 0 0 0 0 0

ValkeyCommandEncoder – Command with 7 words metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 750
pull_request 0 0 0 0 0 0 0 745
Δ 0 0 0 0 0 0 0 -5
Improvement % 0 0 0 0 0 0 0 -5

ValkeyCommandEncoder – Simple GET metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 1885
pull_request 0 0 0 0 0 0 0 1909
Δ 0 0 0 0 0 0 0 24
Improvement % 0 0 0 0 0 0 0 24

ValkeyCommandEncoder – Simple MGET 15 keys metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 360
pull_request 0 0 0 0 0 0 0 362
Δ 0 0 0 0 0 0 0 2
Improvement % 0 0 0 0 0 0 0 2

@adam-fowler
Add testSCRIPTfunctions
0a1d78c 
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
@adam-fowler
Fix testFUNCTIONLIST
fd17b80 
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
@adam-fowler
Add response override for FUNCTION STATS
bdb9314 
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
@adam-fowler
Add command tests for scripts
66e672a 
Add testCommandEncodesDecodes helper function

Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
@adam-fowler adam-fowler marked this pull request as ready for review September 13, 2025 14:31
@adam-fowler adam-fowler added this to the 1.0 milestone Sep 25, 2025
nilanshu-sharma
nilanshu-sharma requested changes Oct 11, 2025
View reviewed changes
Copy link
Collaborator

@nilanshu-sharma nilanshu-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments.

Sources/Valkey/Commands/ScriptingCommands.swift
public func functionLoad<FunctionCode: RESPStringRenderable>(
replace: Bool = false,
functionCode: FunctionCode
) async throws -> FUNCTION.LOADResponse {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ensure consistency in readability, should we call it FUNCTION.LOAD.Response ?
And Similarly for SCRIPT based methods below. Instead of returning SCRIPT.EXISTSResponse, should we return SCRIPT.EXISTS.Response ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I do this is to avoid the generic parameter that comes with FUNCTION.LOAD. If I returned a type that was internal to FUNCTION.LOAD it would require the generic parameter ie FUNCTION.LOAD<FunctionCode>.RESPONSE. Given the response doesn't use the generic parameter FunctionCode it doesn't make sense it include it.

@adam-fowler adam-fowler enabled auto-merge (squash) October 13, 2025 07:25
@adam-fowler adam-fowler merged commit 42c7ac0 into main Oct 13, 2025
12 of 13 checks passed
@adam-fowler adam-fowler deleted the custom-script-functions branch October 13, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nilanshu-sharma nilanshu-sharma nilanshu-sharma approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@adam-fowler @nilanshu-sharma